use options resolver where applicable, fix decoder plugin gzip handling #45
Conversation
|
Huge 👍 |
|
👍 |
use options resolver where applicable, fix decoder plugin gzip handling
|
|
||
| if ($this->useContentEncoding) { | ||
| $request = $request->withHeader('Accept-Encoding', ['gzip', 'deflate', 'compress']); | ||
| if ($this->useContentEncoding && count($encodings)) { |
There was a problem hiding this comment.
is it correct that servers that respect Accept-Encoding will understand us when not sending this header? or should we send an empty header or something to say we can't handle encodings?
There was a problem hiding this comment.
Yes, from the RFC :
If the Accept-Encoding field-value is empty, then only the "identity" encoding is acceptable.
And "identity" encoding = no encoding
There was a problem hiding this comment.
Speak to quick
If no Accept-Encoding field is present in a request, the server MAY assume that the client will accept any content coding. In this case, if "identity" is one of the available content-codings, then the server SHOULD use the "identity" content-coding, unless it has additional information that a different content-coding is meaningful to the client.
There was a problem hiding this comment.
So we should set a default header IMO and add identity to the encoding list.
follow up of the discussion in php-http/documentation#76 - i propose to use the OptionsResolver for configuration options on plugins. This is both more readable on the calling side and more flexible when we need to add more options in the future.
i mixed in the same PR a fix for the deccoder plugin to not claim to support gzip if it does not support gzip. this might hide the problem of a missing gzip library a bit.